GH-49538: [C++][FlightRPC][ODBC] Use static linkage in Windows FlightSQL ODBC driver#49585
GH-49538: [C++][FlightRPC][ODBC] Use static linkage in Windows FlightSQL ODBC driver#49585amoeba wants to merge 39 commits into
Conversation
|
Thanks for opening a pull request! If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename the pull request title in the following format? or See also: |
|
I have this building locally. Before this PR: After: |
74eb8f6 to
3320df2
Compare
ca55ec8 to
7782c69
Compare
|
|
alinaliBQ
left a comment
There was a problem hiding this comment.
@amoeba I see the branch is about a week old, could we rebase on the latest main? I see there are no conflicts, but I am curious to see if the test code changes at odbc_impl in this PR can incidentally help with #49767 (not required for this PR)
I have tested the ODBC MSI installer manually, and can confirm I am able to retrieve data from Power BI using the installer and static linkage ODBC.
alinaliBQ
left a comment
There was a problem hiding this comment.
I was able to build and test release ODBC on my Windows machine locally. But I am getting this error on debug:
I have built from scratch and kept getting this error after several tries. I think this error was from a pre-existing issue where static Debug mode on MSVC was not supported.
absl_synchronization.lib(mutex.cc.obj) : error LNK2005: "private: void __cdecl absl::lts_20250814::Mutex::Dtor(void)" (?Dtor
@Mutex@lts_20250814@absl@@AEAAXXZ) already defined in arrow_flight_static.lib(grpc_client.obj) [C:\path\to\arrow\cpp\static-debug-build\src\arrow\flight\sql\arrow-flight-sql-test.vcxproj]
C:\path\to\arrow\cpp\static-debug-build\debug\Debug\arrow-flight-sql-test.exe :
fatal error LNK1169: one or more multiply defined symbols found [C:\path\to\arrow\cpp\static-debug-build\src\arrow\flight\sql\arrow-flight-sql-test.vcxproj]
Building Custom Rule C:/path/to/arrow/cpp/src/arrow/flight/CMakeLists.txt
test_server.cc
absl_synchronization.lib(mutex.cc.obj) : error LNK2005: "private: void __cdecl absl::lts_20250814::Mutex::Dtor(void)" (?Dtor
@Mutex@lts_20250814@absl@@AEAAXXZ) already defined in arrow_flight_static.lib(protocol_grpc_internal.obj) [C:\path\to\arrow\cpp\static-debug-build\src\arrow\flight\flight-test-server.vcxproj]
C:\path\to\arrow\cpp\static-debug-build\debug\Debug\flight-test-server.exe : fatal error
LNK1169: one or more multiply defined symbols found [C:\path\to\arrow\cpp\static-debug-build\src\arrow\flight\flight-test-server.vcxproj]
(and many errors that look similar)
Since #48964 changes absl bundling, maybe rebasing to main will fix this error on debug static builds.
1f8181f to
56ca928
Compare
|
Thanks for testing @alinaliBQ. I just rebased and I'll try a debug build locally and report back. |
2a77336 to
710ed57
Compare
| void ODBCTestBase::TearDown() { | ||
| ASSERT_EQ(SQL_SUCCESS, SQLFreeHandle(SQL_HANDLE_STMT, stmt)); | ||
| if (connected) { | ||
| ASSERT_EQ(SQL_SUCCESS, SQLFreeHandle(SQL_HANDLE_STMT, stmt)); | ||
| } | ||
| } |
| void ODBCTestBase::TearDownTestSuite() { | ||
| if (connected) { | ||
| // WORKAROUND GH-49585: Skip Disconnect() to avoid segfault when run through CTest | ||
| // | ||
| // When tests are run through CTest (but NOT when run directly from cmd.exe), | ||
| // SQLFreeHandle(SQL_HANDLE_ENV) crashes during cleanup. The crash occurs only with | ||
| // static linkage (ARROW_TEST_LINKAGE=static) and happens while freeing the ODBC | ||
| // environment handle, which triggers cleanup of statically-linked gRPC/Flight | ||
| // resources. | ||
| // | ||
| // Root cause appears to be CTest's process management (signal handling, I/O | ||
| // redirection, or environment differences) interfering with gRPC cleanup during | ||
| // static destruction. | ||
| // | ||
| // This workaround leaks the ODBC handles but allows tests to complete successfully. | ||
| // A proper fix would require ensuring gRPC is kept alive until all ODBC handles are | ||
| // freed, or switching to dynamic linkage for tests. | ||
| // | ||
| // Disconnect(); | ||
| connected = false; | ||
| } | ||
| } |
| @@ -91,8 +102,6 @@ add_arrow_lib(arrow_flight_sql_odbc | |||
| ${ARROW_FLIGHT_SQL_ODBC_SRCS} | |||
| DEFINITIONS | |||
| UNICODE | |||
| if(WIN32 | ||
| AND CMAKE_BUILD_TYPE STREQUAL "Debug" | ||
| AND ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static") | ||
| foreach(LIB_TARGET ${ARROW_FLIGHT_SQL_ODBC_LIBRARIES}) | ||
| target_link_options(${LIB_TARGET} PRIVATE "/FORCE:MULTIPLE") | ||
| endforeach() | ||
| endif() |
| if(WIN32 | ||
| AND CMAKE_BUILD_TYPE STREQUAL "Debug" | ||
| AND ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static") | ||
| if(TARGET arrow_odbc_spi_impl_cli) | ||
| target_link_options(arrow_odbc_spi_impl_cli PRIVATE "/FORCE:MULTIPLE") | ||
| endif() | ||
| if(TARGET arrow-odbc-spi-impl-test) | ||
| target_link_options(arrow-odbc-spi-impl-test PRIVATE "/FORCE:MULTIPLE") | ||
| endif() | ||
| endif() |
| if(WIN32 | ||
| AND CMAKE_BUILD_TYPE STREQUAL "Debug" | ||
| AND ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static") | ||
| foreach(LIB_TARGET ${ARROW_FLIGHT_SQL_LIBRARIES}) | ||
| target_link_options(${LIB_TARGET} PRIVATE "/FORCE:MULTIPLE") | ||
| endforeach() | ||
| endif() |
| if(WIN32 | ||
| AND CMAKE_BUILD_TYPE STREQUAL "Debug" | ||
| AND ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static") | ||
| foreach(TEST_TARGET arrow-flight-sql-test flight-sql-test-server flight-sql-test-app) | ||
| if(TARGET ${TEST_TARGET}) | ||
| target_link_options(${TEST_TARGET} PRIVATE "/FORCE:MULTIPLE") | ||
| endif() | ||
| endforeach() | ||
| endif() |
| if(WIN32 | ||
| AND CMAKE_BUILD_TYPE STREQUAL "Debug" | ||
| AND ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static") | ||
| foreach(LIB_TARGET ${ARROW_FLIGHT_LIBRARIES}) | ||
| target_link_options(${LIB_TARGET} PRIVATE "/FORCE:MULTIPLE") | ||
| endforeach() | ||
| endif() |
| if(WIN32 | ||
| AND CMAKE_BUILD_TYPE STREQUAL "Debug" | ||
| AND ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static") | ||
| set(FLIGHT_TEST_EXES | ||
| arrow-flight-internals-test | ||
| arrow-flight-test | ||
| flight-test-server | ||
| arrow-flight-perf-server | ||
| arrow-flight-benchmark) | ||
| foreach(test_exe ${FLIGHT_TEST_EXES}) | ||
| if(TARGET ${test_exe}) | ||
| target_link_options(${test_exe} PRIVATE "/FORCE:MULTIPLE") | ||
| endif() | ||
| endforeach() | ||
| endif() |
| ARROW_DEPENDENCY_USE_SHARED: OFF | ||
| ARROW_MIMALLOC: OFF | ||
| ARROW_TEST_LINKAGE: shared | ||
| ARROW_USE_STATIC_CRT: OFF |
|
I rebased and will work on cleaning this up this week. |
Rationale for this change
The FlightSQL ODBC driver was previously using dynamic linkage for the Arrow deps it relies on. This worked but prevented a challenge for code signing because (I think) we'd have to sign each DLL and signing costs money. We can use static linkage with a dynamically linked MSVC runtime so we only have one DLL to sign.
Ref: #49404
What changes are included in this PR?
Updates to CMake and CI for the build to allow static linkage with dynamic MSVC runtime (
x64-windows-static-md).Are these changes tested?
Yes.
Are there any user-facing changes?
No.